Skip to content

fix(repository): validate configured remotes before cache access (BUG-001) - #296

Draft
sbalabanov wants to merge 1 commit into
mainfrom
sbalabanov/bug-001
Draft

fix(repository): validate configured remotes before cache access (BUG-001)#296
sbalabanov wants to merge 1 commit into
mainfrom
sbalabanov/bug-001

Conversation

@sbalabanov

@sbalabanov sbalabanov commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Validate repository remotes before they can select cache or workspace state.

  • Require request remotes to exactly match a configured repository before cache access.
  • Use a safe repository-name hint plus a hash of the exact remote for cache and workspace namespaces.
  • Reject ambiguous or unsafe configuration, including credentials, URL queries/fragments, control characters, and relative local paths.
  • Keep clone and worker paths contained under the configured workspace root.

BUG-001 example

If Tango is configured for git@github.com:uber/tango.git but a request supplies an unconfigured value such as ../../outside, the old flow could derive cache or workspace identifiers from the request before confirming it was configured. The new flow rejects that request as an unknown repository before cache access; only the exact configured remote can reach repository state.

Test Plan

  • go test -count=1 ./config ./internal/url ./core/repomanager ./core/cachekey ./controller ./orchestrator/...
  • ./tools/bazel test //config:config_test //internal/url:url_test //core/repomanager:repomanager_test //core/cachekey:cachekey_test //controller:controller_test //orchestrator:orchestrator_test --nocache_test_results --test_output=errors

Revert Plan

Revert the commit.

Issues

T3-BUG-001

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@sbalabanov
sbalabanov marked this pull request as ready for review August 26, 2026 18:32
@sbalabanov
sbalabanov requested review from a team as code owners August 26, 2026 18:32
@sbalabanov
sbalabanov marked this pull request as draft August 26, 2026 19:17
@sbalabanov sbalabanov changed the title fix(repository): hash canonical remotes for BUG-001 fix(repository): validate configured remotes before cache access Aug 27, 2026
@sbalabanov sbalabanov changed the title fix(repository): validate configured remotes before cache access fix(repository): validate configured remotes before cache access (BUG-001) Aug 27, 2026
@sbalabanov

Copy link
Copy Markdown
Contributor Author

make a PR description relevant to the diff between the latest change and a base revision. Simplify wording. Include use case example showing the bug.

Comment thread core/bazel/query.go Outdated
Comment thread config/config.go Outdated
Comment thread core/repomanager/repo_manager_test.go Outdated
Comment thread internal/url/url.go Outdated
@sbalabanov

Copy link
Copy Markdown
Contributor Author

Updated the PR description for the current diff in 2f5946b. It now uses simpler wording and includes a concrete example where GitHub and GitLab remotes with the same repository path previously collided in cache and workspace namespaces.

[addressed by agent]

@sbalabanov

Copy link
Copy Markdown
Contributor Author

Updated the PR description for the current diff against main and added a concrete BUG-001 example showing an unconfigured relative remote being rejected before cache or workspace access. Final SHA: 2f5946b.

[addressed by agent]

Resolve BUG-001 without canonicalizing client-provided repository values.

- require exact configured remote matches before controller cache reads
- hash exact configured values for cache, workspace, and metric identities
- reject credential-bearing and ambiguous repository URL configuration
- retain clone-root containment checks
- remove the unrelated Bazel timeout change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants